Normalize hyphens to underscores in crate names
authorAlex Crichton <alex@alexcrichton.com>
Fri, 20 Mar 2015 20:40:27 +0000 (13:40 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Wed, 25 Mar 2015 02:30:02 +0000 (19:30 -0700)
This change allows *packages* to have hyphens in them, but they are always
translated to underscores when translated to a crate name. This means that all
crates are compiled with a `--crate-name` that has no hyphens (as well as
`--extern` directives having no hyphens).

Binaries, examples, benchmarks, and tests, however, are allowed to contain
hyphens in their name. The "crate name" will still have an underscore, but the
output will be in the same dasherized name.

Explicitly named targets are not allowed to have hyphens in them as well.

src/cargo/core/manifest.rs
src/cargo/ops/cargo_rustc/context.rs
src/cargo/ops/cargo_rustc/custom_build.rs
src/cargo/ops/cargo_rustc/mod.rs
src/cargo/util/toml.rs
tests/test_cargo_compile.rs
tests/test_cargo_compile_custom_build.rs

index f0ce165f8c0741dc5231815d84b00eb17b5ed51e..552455acc0f65e5ab19dad63a9d7a0b32d2b8753 100644 (file)
@@ -322,6 +322,7 @@ impl Target {
     }
 
     pub fn name(&self) -> &str { &self.name }
+    pub fn crate_name(&self) -> String { self.name.replace("-", "_") }
     pub fn src_path(&self) -> &Path { &self.src_path }
     pub fn metadata(&self) -> Option<&Metadata> { self.metadata.as_ref() }
     pub fn kind(&self) -> &TargetKind { &self.kind }
@@ -332,6 +333,10 @@ impl Target {
     pub fn for_host(&self) -> bool { self.for_host }
     pub fn benched(&self) -> bool { self.benched }
 
+    pub fn allows_underscores(&self) -> bool {
+        self.is_bin() || self.is_example() || self.is_custom_build()
+    }
+
     pub fn is_lib(&self) -> bool {
         match self.kind {
             TargetKind::Lib(_) => true,
index 87e1092e3ca7b9626e300dbafc4cf61bfdaf4184..18ce154102e818c0eb44d1da2a71f8558a913f4a 100644 (file)
@@ -282,9 +282,10 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
     /// Returns the file stem for a given target/profile combo
     pub fn file_stem(&self, target: &Target, profile: &Profile) -> String {
         match self.target_metadata(target, profile) {
-            Some(ref metadata) => format!("{}{}", target.name(),
+            Some(ref metadata) => format!("{}{}", target.crate_name(),
                                           metadata.extra_filename),
-            None => target.name().to_string(),
+            None if target.allows_underscores() => target.name().to_string(),
+            None => target.crate_name().to_string(),
         }
     }
 
index 82ea558583eebebd4c6f0507bdf932c43987efa3..6e3d32711b53b419c27fe4f8995da7e984a7ba42 100644 (file)
@@ -46,9 +46,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform,
     };
 
     // Building the command to execute
-    let profile = cx.build_script_profile(pkg.package_id());
-    let to_exec = try!(cx.target_filenames(target, profile))[0].clone();
-    let to_exec = script_output.join(&to_exec);
+    let to_exec = script_output.join(target.name());
 
     // Start preparing the process to execute, starting out with some
     // environment variables. Note that the profile-related environment
index d32d7593df0f96f0411e932230da81212f03d0ec..542eeb1236ec6bbfeb662d7216d89b02911ad970 100644 (file)
@@ -328,9 +328,15 @@ fn rustc(package: &Package, target: &Target, profile: &Profile,
         let pass_l_flag = target.is_lib() || !package.targets().iter().any(|t| {
             t.is_lib()
         });
+        let do_rename = target.allows_underscores() && !profile.test;
+        let real_name = target.name().to_string();
+        let crate_name = target.crate_name();
 
-        let rustc_dep_info_loc = root.join(&cx.file_stem(target, profile))
-                                     .with_extension("d");
+        let rustc_dep_info_loc = if do_rename {
+            root.join(&crate_name)
+        } else {
+            root.join(&cx.file_stem(target, profile))
+        }.with_extension("d");
         let dep_info_loc = fingerprint::dep_info_loc(cx, package, target,
                                                      profile, kind);
         let cwd = cx.config.cwd().to_path_buf();
@@ -363,7 +369,20 @@ fn rustc(package: &Package, target: &Target, profile: &Profile,
                 human(format!("Could not compile `{}`.", name))
             }));
 
-            try!(fs::rename(&rustc_dep_info_loc, &dep_info_loc));
+            if do_rename && real_name != crate_name {
+                let dst = root.join(&filenames[0]);
+                let src = dst.with_file_name(dst.file_name().unwrap()
+                                                .to_str().unwrap()
+                                                .replace(&real_name, &crate_name));
+                try!(fs::rename(&src, &dst).chain_error(|| {
+                    human(format!("could not rename crate {:?}", src))
+                }));
+            }
+
+            try!(fs::rename(&rustc_dep_info_loc, &dep_info_loc).chain_error(|| {
+                human(format!("could not rename dep info: {:?}",
+                              rustc_dep_info_loc))
+            }));
             try!(fingerprint::append_current_dir(&dep_info_loc, &cwd));
 
             Ok(())
@@ -482,7 +501,7 @@ fn rustdoc(package: &Package, target: &Target, profile: &Profile,
     rustdoc.arg(&root_path(cx, package, target))
            .cwd(cx.config.cwd())
            .arg("-o").arg(&cx_root)
-           .arg("--crate-name").arg(target.name());
+           .arg("--crate-name").arg(&target.crate_name());
 
     match cx.resolve.features(package.package_id()) {
         Some(features) => {
@@ -565,7 +584,7 @@ fn build_base_args(cx: &Context,
     // TODO: Handle errors in converting paths into args
     cmd.arg(&root_path(cx, pkg, target));
 
-    cmd.arg("--crate-name").arg(target.name());
+    cmd.arg("--crate-name").arg(&target.crate_name());
 
     for crate_type in crate_types.iter() {
         cmd.arg("--crate-type").arg(crate_type);
@@ -696,7 +715,7 @@ fn build_deps_args(cmd: &mut CommandPrototype,
         for filename in try!(cx.target_filenames(target, profile)).iter() {
             if filename.ends_with(".a") { continue }
             let mut v = OsString::new();
-            v.push(target.name());
+            v.push(&target.crate_name());
             v.push("=");
             v.push(layout.root());
             v.push(&path::MAIN_SEPARATOR.to_string());
index 52148d4ce032863b809d31e7e353e52538351c05..23d255b79b5cc791e3cb94e27d5d58501a3fb2e3 100644 (file)
@@ -395,16 +395,17 @@ impl TomlManifest {
                     ManyOrOne::Many(..) => used_deprecated_lib = true,
                     _ => {}
                 }
-                libs.as_slice().iter().map(|t| {
-                    if layout.lib.is_some() && t.path.is_none() {
+                try!(libs.as_slice().iter().map(|t| {
+                    try!(validate_target_name(t));
+                    Ok(if layout.lib.is_some() && t.path.is_none() {
                         TomlTarget {
                             path: layout.lib.as_ref().map(|p| PathValue::Path(p.clone())),
                             .. t.clone()
                         }
                     } else {
                         t.clone()
-                    }
-                }).collect()
+                    })
+                }).collect::<CargoResult<Vec<_>>>())
             }
             None => inferred_lib_target(&project.name, layout),
         };
@@ -413,34 +414,48 @@ impl TomlManifest {
             Some(ref bins) => {
                 let bin = layout.main();
 
-                bins.iter().map(|t| {
-                    if bin.is_some() && t.path.is_none() {
+                try!(bins.iter().map(|t| {
+                    try!(validate_target_name(t));
+                    Ok(if bin.is_some() && t.path.is_none() {
                         TomlTarget {
                             path: bin.as_ref().map(|&p| PathValue::Path(p.clone())),
                             .. t.clone()
                         }
                     } else {
                         t.clone()
-                    }
-                }).collect()
+                    })
+                }).collect::<CargoResult<Vec<_>>>())
             }
             None => inferred_bin_targets(&project.name, layout)
         };
 
         let examples = match self.example {
-            Some(ref examples) => examples.clone(),
+            Some(ref examples) => {
+                for example in examples {
+                    try!(validate_target_name(example));
+                }
+                examples.clone()
+            }
             None => inferred_example_targets(layout),
         };
 
         let tests = match self.test {
-            Some(ref tests) => tests.clone(),
+            Some(ref tests) => {
+                for test in tests {
+                    try!(validate_target_name(test));
+                }
+                tests.clone()
+            }
             None => inferred_test_targets(layout),
         };
 
         let benches = if self.bench.is_none() || self.bench.as_ref().unwrap().is_empty() {
             inferred_bench_targets(layout)
         } else {
-            self.bench.as_ref().unwrap().iter().map(|t| t.clone()).collect()
+            try!(self.bench.as_ref().unwrap().iter().map(|t| {
+                try!(validate_target_name(t));
+                Ok(t.clone())
+            }).collect::<CargoResult<Vec<_>>>())
         };
 
         // processing the custom build script
@@ -529,6 +544,15 @@ impl TomlManifest {
     }
 }
 
+fn validate_target_name(target: &TomlTarget) -> CargoResult<()> {
+    if target.name.contains("-") {
+        Err(human(format!("target names cannot contain hyphens: {}",
+                          target.name)))
+    } else {
+        Ok(())
+    }
+}
+
 fn process_dependencies<F>(cx: &mut Context,
                            new_deps: Option<&HashMap<String, TomlDependency>>,
                            mut f: F) -> CargoResult<()>
index a78c478e954812e3ba286898197a99ec3156fc7d..e200af5bd887612ae51d7e2f13bc2d280a952a90 100644 (file)
@@ -1653,3 +1653,37 @@ test!(cyclic_deps_rejected {
 cyclic package dependency: package `foo v0.0.1 ([..])` depends on itself
 "));
 });
+
+test!(dashes_to_underscores {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo-bar"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "extern crate foo_bar; fn main() {}");
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(0));
+    assert_that(&p.bin("foo-bar"), existing_file());
+});
+
+test!(dashes_in_crate_name_bad {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [lib]
+            name = "foo-bar"
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "extern crate foo_bar; fn main() {}");
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(101));
+});
index 8745d409c711843ad382251a1b28a882686abb52..093942e866fd6a50c6a9f85b0efa54cd9b321def 100644 (file)
@@ -33,7 +33,7 @@ test!(custom_build_script_failed {
                 execs().with_status(101)
                        .with_stdout(format!("\
 {compiling} foo v0.5.0 ({url})
-{running} `rustc build.rs --crate-name build-script-build --crate-type bin [..]`
+{running} `rustc build.rs --crate-name build_script_build --crate-type bin [..]`
 {running} `[..]build-script-build[..]`
 ",
 url = p.url(), compiling = COMPILING, running = RUNNING))
@@ -760,7 +760,7 @@ test!(build_cmd_with_a_build_cmd {
     --out-dir [..]target[..]deps --emit=dep-info,link \
     -L [..]target[..]deps -L [..]target[..]deps`
 {compiling} foo v0.5.0 (file://[..])
-{running} `rustc build.rs --crate-name build-script-build --crate-type bin \
+{running} `rustc build.rs --crate-name build_script_build --crate-type bin \
     -C prefer-dynamic -g \
     --out-dir [..]build[..]foo-[..] --emit=dep-info,link \
     -L [..]target[..]debug -L [..]target[..]deps \